-
-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Feat: Offer more forking options and test docs #1510
✨ Feat: Offer more forking options and test docs #1510
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 22d5afe The changes in this PR will be included in the next version bump. This PR includes changesets to release 59 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (36)
⛔ Files ignored due to path filters (92)
You can disable this status message by setting the WalkthroughThis pull request introduces a new feature to support viem style transports across the Tevm framework. Multiple packages have been updated to handle transports flexibly by conditionally extracting the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant F as Function
participant T as Transport Provider
C->>F: Call with options/fork with transport
alt Transport is a function
F->>T: Call transport({})
T-->>F: Return { request }
else Transport is an object
F-->>F: Access transport.request directly
end
F-->>C: Return configured Tevm Node / execute fetcher with proper transport
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/state/src/state-types/ForkOptions.ts (1)
2-2
: LGTM! Consider adding JSDoc comments.The changes correctly expand transport support to include both EIP1193 and viem style transports. Consider adding JSDoc comments to document the supported transport types and provide usage examples.
import { type EIP1193RequestFn, type Transport } from 'viem' export interface ForkOptions { + /** + * Transport configuration for forking + * @example + * // Using EIP1193 transport + * transport: { request: myEIP1193RequestFn } + * // Using viem transport + * transport: myViemTransport + */ transport: { request: EIP1193RequestFn } | Transport blockTag?: BlockTag | bigint }Also applies to: 5-5
packages/node/src/getChainId.js (1)
5-5
: LGTM! Consider enhancing error handling.The changes correctly handle both transport types. Consider adding error handling for invalid transport types and documenting the expected transport format.
/** * @param {{request: import('viem').EIP1193RequestFn} | import('viem').Transport} client + * @throws {Error} When transport is invalid or missing request method */ export const getChainId = async (client) => { + if (!client) { + throw new Error('Client is required') + } const transport = typeof client === 'function' ? client({}) : client + if (!transport.request || typeof transport.request !== 'function') { + throw new Error('Invalid transport: missing request method') + } const fetcher = createJsonRpcFetcher(transport)Also applies to: 8-9
packages/node/src/getBlockNumber.js (1)
5-5
: Consider extracting common transport handling logic.The transport handling logic is duplicated across
getBlockNumber.js
andgetChainId.js
. Consider extracting this into a shared utility function.Create a new utility file
packages/node/src/utils/createTransportFetcher.js
:/** * Creates a JSON-RPC fetcher from a transport * @param {{request: import('viem').EIP1193RequestFn} | import('viem').Transport} client * @throws {Error} When transport is invalid or missing request method */ export const createTransportFetcher = (client) => { if (!client) { throw new Error('Client is required') } const transport = typeof client === 'function' ? client({}) : client if (!transport.request || typeof transport.request !== 'function') { throw new Error('Invalid transport: missing request method') } return createJsonRpcFetcher(transport) }Then update both files to use this utility:
- const transport = typeof client === 'function' ? client({}) : client - const fetcher = createJsonRpcFetcher(transport) + const fetcher = createTransportFetcher(client)Also applies to: 8-9
packages/state/src/actions/getForkClient.js (1)
31-33
: Consider enhancing error handling for transport request extraction.The transport request extraction could benefit from additional error handling to ensure the request property exists.
- request: typeof fork.transport === 'function' - ? fork.transport({}).request - : fork.transport.request, + request: (() => { + const transport = typeof fork.transport === 'function' + ? fork.transport({}) + : fork.transport + if (!transport?.request || typeof transport.request !== 'function') { + throw new Error('Invalid fork transport: missing request method') + } + return transport.request + })(),packages/node/src/createTevmNode.js (1)
389-395
: Consider adding runtime type validation.While the implementation is correct, consider adding runtime validation to ensure the transport function returns an object with a request method.
...(options.fork?.transport ? { forkTransport: { request: typeof options.fork.transport === 'function' - ? /** @type {import('viem').Transport} */ (options.fork.transport)({}).request + ? (() => { + const transport = options.fork.transport({}); + if (!transport || typeof transport.request !== 'function') { + throw new Error('Transport function must return an object with a request method'); + } + return transport.request; + })() : options.fork.transport.request } } : {}),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/smart-buses-brake.md
(1 hunks)docs/node/package.json
(1 hunks)packages/node/src/createTevmNode.js
(3 hunks)packages/node/src/getBlockNumber.js
(1 hunks)packages/node/src/getChainId.js
(1 hunks)packages/state/src/actions/getForkClient.js
(1 hunks)packages/state/src/state-types/ForkOptions.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Nx Cloud Agents (6)
- GitHub Check: Nx Cloud Agents (5)
- GitHub Check: Nx Cloud Agents (4)
- GitHub Check: Nx Cloud Agents (3)
- GitHub Check: Nx Cloud Agents (2)
- GitHub Check: Nx Cloud Agents (1)
- GitHub Check: Nx Cloud - Main Job
🔇 Additional comments (4)
docs/node/package.json (1)
16-16
: New Workspace Dependency DeclarationThe addition of
"tevm": "workspace:^"
is clear and correctly placed within the dependencies block. This declaration ensures that the workspace version of the Tevm package is used. Please verify that your build system and package manager fully support theworkspace:
protocol, and that the version resolution aligns with your monorepo setup.packages/node/src/createTevmNode.js (2)
145-149
: LGTM: Clean implementation of viem transport support.The changes correctly handle both function-style and object-style transports, ensuring consistent access to the request method.
342-346
: LGTM: Consistent deep copy implementation for fork transport.The changes correctly preserve the fork transport during deep copy operations, maintaining consistency with the transport handling pattern.
.changeset/smart-buses-brake.md (1)
1-64
: LGTM: Appropriate version bumps and clear description.The patch version bumps are appropriate for the feature addition, and the description clearly communicates the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
docs/node/docs/pages/advanced/custom-precompiles.test.ts (2)
101-111
: 🛠️ Refactor suggestionImprove error handling in the gas calculation precompile.
Instead of throwing an error, consider returning an
EvmError
for consistency with other precompiles.call: async ({ data, gasLimit }) => { // Charge 100 gas per byte const gasUsed = BigInt(data.length * 100) if (gasUsed > gasLimit) { - throw new Error('Out of gas') + return { + returnValue: new Uint8Array(), + exceptionError: new EvmError(EvmErrorMessage.OUT_OF_GAS), + executionGasUsed: gasLimit, + } } return {
138-146
: 🛠️ Refactor suggestionConsider using EvmError for error handling.
For consistency with other error handling patterns, consider using
EvmError
instead of throwing a generic Error.call: async ({ data }) => { if (data.length === 0) { - throw new Error('Empty input not allowed') + return { + returnValue: new Uint8Array(), + exceptionError: new EvmError(EvmErrorMessage.INVALID_INPUT), + executionGasUsed: 200n, + } } return {
🧹 Nitpick comments (16)
docs/node/docs/pages/advanced/custom-precompiles.test.ts (2)
17-24
: Consider adding input validation.While the basic precompile implementation is correct, it might be good to add input validation to handle edge cases.
call: async ({ data}) => { + if (!data || data.length === 0) { + return { + returnValue: new Uint8Array(), + executionGasUsed: 200n, + } + } // Simple precompile that doubles each byte const input = Array.from(data) return {
51-72
: Enhance gas limit handling in the state precompile.The gas limit check is correctly implemented, but consider adding validation for the input data length to ensure it matches the expected format (32 bytes for key and 32 bytes for value).
call: async ({ data, gasLimit }) => { + if (data.length !== 64) { + return { + returnValue: new Uint8Array(), + exceptionError: new EvmError(EvmErrorMessage.INVALID_INPUT_LENGTH), + executionGasUsed: 200n, + } + } const key = data.slice(0, 32) as Hex const value = data.slice(32) as Hexdocs/node/docs/pages/advanced/custom-precompiles.mdx (2)
73-87
: Consider documenting gas calculation methodology.While the gas handling is correctly implemented, it would be helpful to document how the gas cost (200n) was determined.
Add a comment explaining the gas calculation:
call: async ({ data, gasLimit }) => { + // Gas cost breakdown: + // - Base cost: 100 + // - Memory expansion: 50 + // - Data processing: 50 const executionGasUsed = 200n
224-228
: Consider expanding best practices documentation.The best practices section could benefit from additional guidance on gas calculation strategies and common pitfalls.
Add the following points to the best practices:
- Document the rationale behind gas costs
- Provide examples of common gas calculation patterns
- List typical gas costs for different operations
docs/node/docs/pages/api/state.mdx (5)
22-46
: Comprehensive StateManager Interface DocumentationThe StateManager interface now includes a broad range of methods covering account handling, contract code/storage, snapshots, and state proofs. This comprehensive list is very useful.
- Recommendation: Please double-check that these method signatures are fully consistent with the underlying implementation. If possible, consider adding brief inline comments or annotations for each method to improve future readability.
52-57
: Creating a State Manager ExampleThe updated code snippet for creating a State Manager is concise and demonstrates the new import path along with a configurable logging level.
- Suggestion: It could be beneficial to list or reference the possible
loggingLevel
options (e.g., 'info', 'debug', etc.) for clarity.
89-97
: Contract Management Example EnhancementThe contract management snippet covers both contract code and storage operations adequately, showing updated usage of
putContractCode
,getContractCode
,putContractStorage
, andgetContractStorage
.
- Consideration: Adding a brief note about error handling for these asynchronous operations might further improve clarity for users.
155-157
: Deep Copy Example in Cache ManagementThe deep copy snippet is a valuable addition that illustrates how to obtain an independent snapshot of the state.
- Suggestion: Consider adding a brief comment about any potential performance implications when using deep copies, especially in high-frequency scenarios.
186-194
: Robust Error Handling ExampleThe error-handling snippet effectively demonstrates how to catch errors when retrieving an account, including handling a potential
AccountNotFoundError
.
- Suggestion: Ensure that
AccountNotFoundError
is clearly defined or imported in the context where this example is used so that users know where it originates.docs/node/docs/pages/core/create-tevm-node.test.ts (2)
46-47
: Separate assertions for better test failure debugging.The combined assertion makes it harder to identify which condition failed. Consider separating the assertions for better error messages.
- expect(node.miningConfig.type === 'interval' && node.miningConfig.interval).toBe(12_000) + expect(node.miningConfig.type).toBe('interval') + expect(node.miningConfig.interval).toBe(12_000)
62-62
: Remove debug console.log statement.Debug logging should not be included in test files as it pollutes test output.
- console.log(data, gasLimit)
docs/node/docs/pages/core/forking.test.ts (2)
110-112
: Replace non-null assertions with proper null checking.Using
!
assertions could mask potential runtime errors. Consider proper null checking instead.- expect(usdcContract!.balance).toBeDefined() - expect(usdcContract!.nonce).toBeDefined() - expect(usdcContract!.codeHash).toBeDefined() + expect(usdcContract?.balance).toBeDefined() + expect(usdcContract?.nonce).toBeDefined() + expect(usdcContract?.codeHash).toBeDefined() - account!.balance += 1000000000000000000n + if (!account) { + throw new Error('Account not found') + } + account.balance += 1000000000000000000nAlso applies to: 126-130
146-157
: Make performance test more robust.Timing-based tests can be flaky due to system load and other factors. Consider using a relative threshold or counting actual network requests instead.
- expect(cachedAccess).toBeLessThan(firstAccess) + // Allow for some variance but ensure significant improvement + expect(cachedAccess).toBeLessThan(firstAccess * 0.5)docs/node/docs/pages/core/create-tevm-node.mdx (1)
141-163
: Enhanced Precompile Registration for Improved Type Safety
The changes in the "Custom Precompiles" section show a clear update: using bothcreateContract
andparseAbi
to define the precompile, and then registering it viamyPrecompile.precompile()
. This enhances readability and type safety. Consider adding a brief note explaining why this approach is preferred over directly assigning an address.docs/node/docs/pages/advanced/txpool.mdx (1)
174-183
: Use of Helper Functions for Transaction Creation
The code snippet for creating transactions now recommends helper functions (parseEther
,gweiToWei
) for value and fee conversions. This enhances type safety and clarity, though ensure that these helper functions are documented elsewhere in the project.docs/node/docs/pages/core/managing-state.mdx (1)
47-61
: Standardizing Account Creation/Modification
Switching toEthjsAccount.fromAccountData
for account creation is a welcome improvement. This approach should be documented as the preferred method over direct instantiation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
docs/node/docs/dist/.vocs/search-index-6b680a3c.json
is excluded by!**/dist/**
docs/node/docs/dist/llms-full.txt
is excluded by!**/dist/**
docs/node/docs/dist/llms.txt
is excluded by!**/dist/**
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
docs/node/docs/pages/advanced/custom-precompiles.mdx
(8 hunks)docs/node/docs/pages/advanced/custom-precompiles.test.ts
(10 hunks)docs/node/docs/pages/advanced/receipts-and-logs.mdx
(4 hunks)docs/node/docs/pages/advanced/receipts-and-logs.test.ts
(7 hunks)docs/node/docs/pages/advanced/txpool.mdx
(1 hunks)docs/node/docs/pages/advanced/txpool.test.ts
(2 hunks)docs/node/docs/pages/api/memory-client.test.ts
(1 hunks)docs/node/docs/pages/api/methods.test.ts
(1 hunks)docs/node/docs/pages/api/state.mdx
(4 hunks)docs/node/docs/pages/api/state.test.ts
(7 hunks)docs/node/docs/pages/api/tevm-call.mdx
(5 hunks)docs/node/docs/pages/api/tevm-call.test.ts
(1 hunks)docs/node/docs/pages/core/create-tevm-node.mdx
(6 hunks)docs/node/docs/pages/core/create-tevm-node.test.ts
(5 hunks)docs/node/docs/pages/core/forking.mdx
(5 hunks)docs/node/docs/pages/core/forking.test.ts
(3 hunks)docs/node/docs/pages/core/managing-state.mdx
(10 hunks)docs/node/docs/pages/core/managing-state.test.ts
(10 hunks)docs/node/docs/pages/core/tevm-node-interface.test.ts
(4 hunks)docs/node/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/node/docs/pages/api/memory-client.test.ts
- docs/node/docs/pages/api/tevm-call.test.ts
- docs/node/docs/pages/api/methods.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/node/package.json
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Nx Cloud Agents (6)
- GitHub Check: Nx Cloud Agents (5)
- GitHub Check: Nx Cloud Agents (4)
- GitHub Check: Nx Cloud Agents (3)
- GitHub Check: Nx Cloud Agents (2)
- GitHub Check: Nx Cloud Agents (1)
- GitHub Check: Nx Cloud - Main Job
🔇 Additional comments (65)
docs/node/docs/pages/advanced/custom-precompiles.test.ts (1)
2-2
: LGTM! Import changes look good.The addition of
EvmError
andEvmErrorMessage
imports aligns with the enhanced error handling approach.Also applies to: 7-7
docs/node/docs/pages/advanced/custom-precompiles.mdx (2)
20-20
: LGTM! Basic example is well documented.The example correctly demonstrates the basic usage of custom precompiles with proper gas handling.
Also applies to: 28-35
253-256
: LGTM! Error handling documentation is comprehensive.The error handling section properly documents the use of
EvmError
types and important checks.docs/node/docs/pages/api/state.mdx (9)
13-14
: Alternative Import Path ClarificationThe new note ("Or use
tevm/state
if you have the maintevm
package installed") clearly informs users of an alternate import path, which is helpful for maintainers who bundle the main package.
19-20
: Interface Introduction UpdateThe updated introductory text ("The main interface for managing Ethereum state:") sets a clear expectation for what follows. It effectively prefaces the interface declaration.
64-68
: Account Management Usage ExampleThe example for account management now uses the updated API calls (e.g.,
getAccount
,putAccount
,deleteAccount
,modifyAccountFields
) correctly. The usage is clear and straightforward.
99-101
: Contract Code Operations ClarityThe updated code segment demonstrates how contract code is set and retrieved, which reinforces the new API behavior.
103-105
: Contract Storage Retrieval ExampleThe snippet correctly shows how to retrieve the contract storage value immediately after setting it. This sequential flow is well demonstrated.
143-144
: State Dumping Range ExampleThe example usage of
dumpStorageRange
now correctly uses a bigint literal (4n) for the start key and a numeric limit. This clarifies the intended argument types.
163-165
: Ensuring Readiness withready()
The "Always await ready()" example is effective in reiterating the importance of awaiting asynchronous initialization.
169-176
: Atomic Operations with CheckpointsThe checkpoint, commit, and revert example is clear and demonstrates how to manage state changes atomically. This practical usage example is helpful for developers dealing with complex state transitions.
180-182
: Cache Clearing Best PracticeThis snippet emphasizes the importance of managing memory usage by clearing caches periodically. It is clear and to the point.
docs/node/docs/pages/api/tevm-call.mdx (12)
1-4
: Front Matter Update
The YAML front matter now clearly demarcates the document metadata with a closing separator on line 4. Please verify that the document renders correctly on the documentation site.
15-16
: Updated Imports Including PREFUNDED_ACCOUNTS
The import statement on line 16 now includesPREFUNDED_ACCOUNTS
along withcreateTevmNode
. This aligns the examples with the new data source for thefrom
address. Ensure thatPREFUNDED_ACCOUNTS
is properly defined and exported from thetevm
package.
21-23
: Updated Call API Example Parameters
The basic usage example now sets thefrom
address toPREFUNDED_ACCOUNTS[0].address
and uses a concrete WETH address forto
, while thedata
parameter is set to an empty call. This improves clarity on how to correctly populate these fields.
95-103
: Added Example ERC20 ABI for balanceOf
The example now includes a concrete ERC20 ABI declaration for thebalanceOf
function. This change removes ambiguity by replacing the previous placeholder and provides a clear, reusable example.
105-110
: Enhanced Function Call with encodeFunctionData
In the example call (lines 105-110), the snippet now explicitly sets theto
andfrom
parameters and leveragesencodeFunctionData
with the updated ABI, function name, and arguments. This change not only improves consistency but also shows a more realistic usage scenario.
114-116
: Clear Result Decoding Example
The snippet usingdecodeFunctionResult
(lines 114-116) now clearly passes the ABI and function name. This explicitness helps users to understand how to correctly decode the call result.
124-126
: Updated Contract Deployment Bytecode
The contract deployment example now provides a specific bytecode string that implements a simple contract (returning 42). Please verify that the provided bytecode is valid and behaves as expected in a deployment scenario.
128-129
: Consistent Deployment Parameters
In the deployment example, thefrom
address is now set toPREFUNDED_ACCOUNTS[0].address
and thedata
field is assigned the explicit bytecode. This ensures consistency with the other examples. Check that thecreateTransaction: true
flag functions as intended.
140-148
: Enhanced State Override Example
The state override example (lines 140-148) now usesPREFUNDED_ACCOUNTS[0].address
for thefrom
parameter and updates the state override mapping with clear values (balance, nonce, code, and state). This makes it easier to simulate specific account states during testing.
158-160
: Updated Debug Trace Call Parameters
The debug trace example now consistently usesPREFUNDED_ACCOUNTS[0].address
for thefrom
address and the specified WETH address forto
. These changes help standardize the examples across the documentation.
165-169
: Conditional Trace Logging Enhancement
The modification on lines 165-169 adds a check forresult.trace
before iterating overstructLogs
, preventing potential runtime errors if the trace is undefined. This condition improves robustness when handling debug traces.
174-174
: Streamlined Higher-Level API Commentary
The trailing line beginning with "WhilecallHandler
" on line 174 has been updated (or truncated) to streamline the documentation. Please confirm that this removal does not omit any essential information needed for understanding higher-level APIs.docs/node/docs/pages/core/create-tevm-node.test.ts (3)
54-60
: LGTM! Enhanced type safety in precompile definition.The use of
createContract
withparseAbi
improves type safety and provides better contract interface definition.
71-71
: LGTM! Improved precompile registration pattern.The use of
myPrecompile.precompile()
provides a clearer API for registering precompiles.
18-18
:❓ Verification inconclusive
Standardize transport initialization pattern.
The transport initialization has been updated to use a consistent pattern with an empty object parameter:
http(url)({})
. This change aligns with viem's transport configuration style.Let's verify this is the recommended pattern in viem's documentation:
Also applies to: 105-105
🌐 Web query:
What is the correct way to initialize HTTP transport in viem?
💡 Result:
To initialize an HTTP transport in Viem correctly:
- Basic Initialization
Importhttp
from viem and use it as your transport provider when creating a client:import { createPublicClient, http } from 'viem' import { mainnet } from 'viem/chains' const client = createPublicClient({ chain: mainnet, transport: http('https://eth-mainnet.g.alchemy.com/v2/YOUR_API_KEY') })
- Key Configuration Options
The HTTP transport supports several optional parameters:const transport = http('https://rpc-url', { batch: true, // Enable Batch JSON-RPC (default false) retryCount: 5, // Max retry attempts (default 3) retryDelay: 100 // Base retry delay in ms (default 150) })
- Authentication Best Practice
Always provide an authenticated RPC endpoint to avoid rate limiting[2][5]. Public nodes should only be used for testing:// Recommended authenticated setup transport: http('https://eth-mainnet.g.alchemy.com/v2/your-api-key') // Avoid public nodes in production transport: http() // Falls back to chain's public RPC
- Batch Request Optimization
Enable batch processing for multiple concurrent requests:const transport = http('https://rpc-url', { batch: true }) // Executes as single batch request const [balance, block] = await Promise.all([ client.getBalance(...), client.getBlock(...) ])
- Full Client Configuration
Combine with other client options for complete setup:import { createWalletClient } from 'viem' const walletClient = createWalletClient({ chain: mainnet, transport: http(), account: privateKeyToAccount('0x...') }) ```[1][4][7] The core initialization requires specifying your JSON-RPC endpoint in the `http()` transport configuration while creating either a public or wallet client[1][5][7]. Batch processing and retry configurations are recommended for production-grade applications[2][9]. Citations: - 1: https://viem.sh/docs/clients/transports/http.html - 2: https://viem-iyn6kgwt-wxqdoit.4everland.app/docs/clients/transports/http - 3: https://docs.devicewise.com/Content/Products/GatewayDevelopersGuide/EnterpriseConnectivity/Defining-an-HTTP-transport.htm - 4: https://viem.sh/docs/clients/custom - 5: https://docs.soneium.org/docs/builders/tutorials/client-integration/viem - 6: https://docs.moonbeam.network/builders/ethereum/libraries/viem/ - 7: https://viem.sh/docs/clients/public.html - 8: https://viem.sh/docs/clients/transports/custom.html - 9: https://viem.sh/docs/clients/transports/websocket - 10: https://code.zksync.io/tutorials/guide-viem --- </details> **Standardize transport initialization pattern.** The transport configuration was updated to use a consistent pattern with an explicit empty options object, as seen with ```typescript transport: http('https://mainnet.infura.io/v3/YOUR-KEY')({})While this pattern standardizes the initialization across the codebase (lines 18 and 105), the latest viem documentation shows that the recommended basic initialization is to invoke
http(...)
directly (or pass configuration as a second argument, e.g.,http('https://rpc-url', { batch: true })
). If no additional configuration is needed, consider using the canonical form:transport: http('https://mainnet.infura.io/v3/YOUR-KEY')Please verify whether the extra invocation with an empty object is intentional for future configurability or if it should be aligned with the documented patterns.
- Affected locations:
docs/node/docs/pages/core/create-tevm-node.test.ts
: Line 18docs/node/docs/pages/core/create-tevm-node.test.ts
: Line 105docs/node/docs/pages/api/state.test.ts (3)
9-11
: LGTM! Simplified state manager initialization.The removal of the
common
parameter simplifies the API while maintaining functionality.Also applies to: 24-24
25-25
: LGTM! Enhanced type safety in address and data handling.The use of
createAddress
andhexToBytes
utility functions improves type safety and standardizes data handling across the codebase.Also applies to: 48-50
109-109
: LGTM! Consistent use of BigInt for block numbers.The use of BigInt for the storage range aligns with the standard practice of using BigInt for block numbers.
docs/node/docs/pages/core/tevm-node-interface.test.ts (3)
26-26
: LGTM! Standardized transaction creation pattern.The use of
createImpersonatedTx
provides a consistent and type-safe way to create transactions for testing.Also applies to: 60-65
71-85
: LGTM! Comprehensive receipt validation.The enhanced receipt validation properly checks for post-Byzantium fields and provides thorough type checking.
92-106
: LGTM! Thorough log structure validation.The enhanced log validation ensures proper structure and includes comprehensive checks for all log components.
docs/node/docs/pages/core/managing-state.test.ts (4)
2-4
: LGTM!The imports are correctly added and align with the changes in the codebase.
23-23
: LGTM! Improved type safety and standardized address handling.The changes enhance type safety by using
createAddress
for address creation andEthjsAccount.fromAccountData
for account data initialization.Also applies to: 38-38
69-70
: LGTM! Enhanced storage handling with proper type conversion.The changes improve type safety by using
hexToBytes
to convert hex strings to byte arrays for storage operations.Also applies to: 78-79
97-111
: LGTM! Consistent implementation of state checkpoint operations.The changes maintain consistency with the codebase's patterns for address handling and storage operations.
docs/node/docs/pages/advanced/txpool.test.ts (4)
11-24
: LGTM! Enhanced transaction handling and pool operations.The changes improve transaction handling by using
createImpersonatedTx
for transaction creation and standardize pool operations withtxPool
methods.Also applies to: 31-56
63-81
: LGTM! Consistent implementation of advanced transaction features.The changes maintain consistency in handling transaction replacement and expiry scenarios.
Also applies to: 93-112
119-131
: LGTM! Improved error handling and validation.The changes enhance error handling for invalid transactions and nonce gaps.
Also applies to: 141-165
173-211
: LGTM! Maintained performance characteristics.The changes preserve the performance characteristics for handling concurrent transactions.
docs/node/docs/pages/advanced/receipts-and-logs.test.ts (4)
8-12
: LGTM! Enhanced code readability with constants.The addition of
CONTRACT_BYTECODE
andEMIT_EVENT_SELECTOR
constants improves code readability and maintainability.
89-132
: LGTM! Comprehensive receipt handling test coverage.The changes provide thorough test coverage for receipt handling with multiple scenarios.
237-269
: LGTM! Comprehensive log filtering test coverage.The changes provide thorough test coverage for log filtering with multiple scenarios.
276-293
: LGTM! Improved error handling test coverage.The changes enhance error handling for non-existent receipts and invalid filters.
docs/node/docs/pages/advanced/receipts-and-logs.mdx (4)
15-44
: LGTM! Clear and comprehensive receipt management examples.The examples effectively demonstrate receipt management with proper error handling and type safety.
71-132
: LGTM! Clear and comprehensive event logs examples.The examples effectively demonstrate contract deployment, event emission, and log querying with multiple filter scenarios.
134-152
: LGTM! Clear error handling examples.The examples effectively demonstrate handling of non-existent receipts and invalid filters.
177-190
: LGTM! Clear best practices examples.The examples effectively demonstrate type safety and proper error handling patterns.
docs/node/docs/pages/core/forking.mdx (4)
25-27
: Ensure Consistent Invocation of the Transport Function
The example now correctly invokes thehttp
function with an empty object (({})
), which aligns with the new requirements. Verify that this pattern is consistently followed elsewhere in the docs.
33-35
: Clarify the Transport Function Requirement in the Callout
The updated callout now clearly states that the transport function must be called with an empty object. This clarifies usage for users familiar with EIP-1193 providers and viem style transports.
46-56
: Verify Reforking Strategy Example
The reforking example correctly demonstrates using the source node as a transport with proper conversion of the current block usinghexToBigInt
. Make sure that users understand that the same pattern should be applied when forking from a live network or another Tevm instance.
83-101
: Usage of Deep Copy with Address Handling
The deep copy example now makes use ofcreateAddress
for improved type safety. This is a good enhancement for clarity and consistency in address management.docs/node/docs/pages/core/create-tevm-node.mdx (2)
85-96
: Updated Mining Configuration Examples
The mining configuration examples now cover both auto and interval-based mining. These examples clearly illustrate the new structured type forminingConfig
(e.g.{ type: 'interval', interval: <number> }
). This change improves clarity for users setting up different node behaviors.
193-197
: Reinforce Best Practice by Awaiting Node Readiness
Includingawait node.ready()
in the best practices section serves as an important reminder to ensure that node initialization completes before proceeding.docs/node/docs/pages/advanced/txpool.mdx (5)
2-4
: Updated Document Title and Description
The new title "Transaction Pool" and the revised description properly reflect the enhanced documentation and functionality updates for managing pending Ethereum transactions.
12-33
: Quick Start Example Using New Transaction Methods
The quick start example now demonstrates the use ofcreateImpersonatedTx
alongside obtaining the TxPool viaawait node.getTxPool()
. This clearly illustrates the intended workflow for quickly setting up and interacting with the transaction pool.
89-104
: Demonstrate Effective Transaction Replacement
The transaction replacement example illustrates replacing an original transaction by submitting a new one with a higher gas fee. The inline comment regarding the requirement for at least a 10% gas price bump is clear and informative.
218-228
: Memory Management and Cleanup Guidance
The inclusion of a cleanup cycle example and pool size monitoring is a valuable addition. This example provides practical guidance on maintaining optimal performance of the transaction pool.
235-248
: Comprehensive API Reference for TxPool
The newly updated API reference section for theTxPool
class now includes all major functions and method signatures. This should help developers quickly understand available actions.docs/node/docs/pages/core/managing-state.mdx (5)
17-22
: Simplified Node and State Manager Initialization
The updated snippet for initializing the node and accessing the state manager is succinct and clear. It demonstrates best practices in setting up the VM for state operations.
29-32
: Improved Address Handling in Account Reading
The replacement of hardcoded addresses withcreateAddress
ensures better type safety. This change modernizes the example and is consistent with updates in other documentation parts.
115-122
: Consistent Use of Address and Storage Formatting Utilities
The examples for reading and writing storage now usecreateAddress
andhexToBytes
fromtevm/utils
, which enhances consistency and clarifies expected data formats.
155-184
: Robust Example for State Checkpoints and Atomic Updates
The checkpoint example demonstrates proper use ofcommit
andrevert
within a try–catch block. This is an excellent illustration of handling atomic state updates. Consider noting any version-specific requirements if applicable.
242-254
: Effective Error-Handling Best Practices
The best practices snippet for error handling in state operations gives clear instructions (e.g., throwing 'Account not found'). This ensures that developers are aware of potential pitfalls and how to handle them gracefully.
it('should modify state in forked node', async () => { | ||
// @ts-ignore TODO: Fix viem version mismatch | ||
const node = createTevmNode({ | ||
fork: { | ||
// @ts-ignore TODO: Fix viem version mismatch | ||
transport: http('https://mainnet.infura.io/v3/YOUR-KEY'), | ||
}, | ||
}) | ||
|
||
// Get USDC contract state | ||
const USDC_ADDRESS = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' | ||
const vm = await node.getVm() | ||
const usdcContract = await vm.stateManager.getAccount(USDC_ADDRESS) | ||
const address = createAddress('0x1234567890123456789012345678901234567890') | ||
const account = await vm.stateManager.getAccount(address) | ||
if (account) { | ||
account.balance += 1000000000000000000n // Add 1 ETH | ||
await vm.stateManager.putAccount(address, account) | ||
} | ||
|
||
expect(usdcContract).toBeDefined() | ||
expect(usdcContract.balance).toBeDefined() | ||
expect(usdcContract.nonce).toBeDefined() | ||
expect(usdcContract.codeHash).toBeDefined() | ||
const updatedAccount = await vm.stateManager.getAccount(address) | ||
expect(updatedAccount?.balance).toBe(account?.balance) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove duplicate test case.
The test case "should modify state in forked node" appears twice with nearly identical implementation. Consider removing one instance to improve maintainability.
Also applies to: 115-131
import { performance } from 'node:perf_hooks' | ||
|
||
describe('Forking', () => { | ||
describe('Basic Forking', () => { | ||
it('should create a forked node', async () => { | ||
it('should create forked node', async () => { | ||
// @ts-ignore TODO: Fix viem version mismatch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Address viem version mismatch.
Multiple TODO comments indicate a version mismatch with viem that needs to be resolved to prevent potential runtime issues.
Let's check the current version constraints:
Also applies to: 26-26, 43-43, 58-58, 78-78, 118-118
🏁 Script executed:
#!/bin/bash
# Check package.json for viem version constraints
cat package.json | jq '.dependencies.viem, .devDependencies.viem'
Length of output: 77
Resolve Viem Version Mismatch
The test file docs/node/docs/pages/core/forking.test.ts
(and similar TODOs at lines 10, 26, 43, 58, 78, 118) still includes comments flagging a viem version mismatch. Our check confirms that viem
is not explicitly defined in package.json
(both dependencies and devDependencies return null
), which means we're likely relying on a transitive version that may not be consistent. To prevent potential runtime issues, please either add an explicit version for viem
in package.json
or adjust the test/code to properly handle the version discrepancy.
d766cf3
to
22d5afe
Compare
Description
Concise description of proposed changes
Testing
Explain the quality checks that have been done on the code changes
Additional Information
Your ENS/address:
Summary by CodeRabbit